[DA] Fix Strong SIV test for symbolic coefficients and deltas (#149977) #157738
+111
−12
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…49977) Fixes GitHub issue llvm#149977 where Strong SIV test incorrectly rejected dependencies with symbolic coefficients and deltas due to overly conservative bound checking. Root cause: The bound constraint check |Delta| > UpperBound * |Coeff| would prematurely reject dependencies when SCEV couldn't prove the relationship definitively for symbolic expressions, preventing the analysis from reaching the division logic. Solution: 1. Make bound check less conservative for symbolic expressions by adding runtime assumptions when SCEV cannot determine the relationship. 2. Enable symbolic division using SE->getUDivExactExpr for Delta/Coeff. 3. Add runtime assumptions where symbolic division cannot be computed. This enables precise dependence analysis for cases like: - Coefficient: -k (symbolic) - Delta: -(2*k + 1) (symbolic) - Distance: (2*k + 1)/k (computed symbolically) Test case validates: - When k = -1: distance = 1, clear flow dependence detected. - Runtime assumptions ensure bounds are satisfied.
llvmbot
added
the
llvm:analysis
Member
|
@llvm/pr-subscribers-llvm-analysis Author: Sebastian Pop (sebpop) ChangesFixes GitHub issue #149977 where Strong SIV test incorrectly rejected Root cause: The bound constraint check |Delta| > UpperBound * |Coeff| would Solution:
This enables precise dependence analysis for cases like:
Test case validates:
Patch is 22.03 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/157738.diff 9 Files Affected:
diff --git a/llvm/include/llvm/Analysis/DependenceAnalysis.h b/llvm/include/llvm/Analysis/DependenceAnalysis.h
index f66c79d915665..300cfb73af5c1 100644
--- a/llvm/include/llvm/Analysis/DependenceAnalysis.h
+++ b/llvm/include/llvm/Analysis/DependenceAnalysis.h
@@ -285,7 +285,8 @@ class LLVM_ABI FullDependence final : public Dependence {
class DependenceInfo {
public:
DependenceInfo(Function *F, AAResults *AA, ScalarEvolution *SE, LoopInfo *LI)
- : AA(AA), SE(SE), LI(LI), F(F) {}
+ : AA(AA), SE(SE), LI(LI), F(F), Assumptions({}, *SE),
+ UnderRuntimeAssumptions(false) {}
/// Handle transitive invalidation when the cached analysis results go away.
LLVM_ABI bool invalidate(Function &F, const PreservedAnalyses &PA,
@@ -355,7 +356,33 @@ class DependenceInfo {
ScalarEvolution *SE;
LoopInfo *LI;
Function *F;
- SmallVector<const SCEVPredicate *, 4> Assumptions;
+
+ /// Runtime assumptions collected during dependence analysis.
+ ///
+ /// The dependence analysis employs a cascade of tests from simple to complex:
+ /// ZIV -> SIV (Strong/Weak-Crossing/Weak-Zero) -> RDIV -> MIV -> Banerjee.
+ /// Each test attempts to characterize the dependence with increasing
+ /// precision.
+ ///
+ /// Assumption Management Strategy:
+ /// - Each test may require runtime assumptions (e.g., "coefficient != 0")
+ /// to provide precise analysis.
+ /// - If UnderRuntimeAssumptions=true: tests can add assumptions and continue.
+ /// - If UnderRuntimeAssumptions=false: tests that need assumptions fail
+ /// gracefully, allowing more complex tests to attempt analysis.
+ /// - Only assumptions from successful tests contribute to the final result.
+ /// - SCEVUnionPredicate automatically deduplicates redundant assumptions.
+ ///
+ /// This design ensures:
+ /// 1. Simpler tests get priority (better performance).
+ /// 2. Complex tests serve as fallbacks when simple tests fail.
+ /// 3. No unnecessary runtime checks from failed test attempts.
+ /// 4. Maintains the intended cascade behavior of the dependence analysis.
+ SCEVUnionPredicate Assumptions;
+
+ /// Indicates whether runtime assumptions are collected during the analysis.
+ /// When false, dependence tests that would require runtime assumptions fail.
+ bool UnderRuntimeAssumptions;
/// Subscript - This private struct represents a pair of subscripts from
/// a pair of potentially multi-dimensional array references. We use a
diff --git a/llvm/lib/Analysis/DependenceAnalysis.cpp b/llvm/lib/Analysis/DependenceAnalysis.cpp
index da86a8d2cc9c0..d1ae9cd4a1bcf 100644
--- a/llvm/lib/Analysis/DependenceAnalysis.cpp
+++ b/llvm/lib/Analysis/DependenceAnalysis.cpp
@@ -1249,10 +1249,33 @@ bool DependenceInfo::strongSIVtest(const SCEV *Coeff, const SCEV *SrcConst,
SE->isKnownNonNegative(Coeff) ? Coeff : SE->getNegativeSCEV(Coeff);
const SCEV *Product = SE->getMulExpr(UpperBound, AbsCoeff);
if (isKnownPredicate(CmpInst::ICMP_SGT, AbsDelta, Product)) {
- // Distance greater than trip count - no dependence
- ++StrongSIVindependence;
- ++StrongSIVsuccesses;
- return true;
+ // Check if this involves symbolic expressions where we might be too
+ // conservative.
+ if (isa<SCEVUnknown>(Delta) || isa<SCEVUnknown>(Coeff) ||
+ !isa<SCEVConstant>(AbsDelta) || !isa<SCEVConstant>(Product)) {
+ // For symbolic expressions, add runtime assumption rather than
+ // rejecting.
+ const SCEVPredicate *BoundPred =
+ SE->getComparePredicate(ICmpInst::ICMP_SLE, AbsDelta, Product);
+ if (UnderRuntimeAssumptions) {
+ SmallVector<const SCEVPredicate *, 4> NewPreds(
+ Assumptions.getPredicates());
+ NewPreds.push_back(BoundPred);
+ const_cast<DependenceInfo *>(this)->Assumptions =
+ SCEVUnionPredicate(NewPreds, *SE);
+ LLVM_DEBUG(dbgs() << "\t Added runtime bound assumption\n");
+ } else {
+ // Cannot add runtime assumptions, let more complex tests try.
+ LLVM_DEBUG(dbgs() << "\t Would need runtime bound assumption but "
+ "not allowed. Failing this test.\n");
+ return false;
+ }
+ } else {
+ // Distance definitely greater than trip count - no dependence
+ ++StrongSIVindependence;
+ ++StrongSIVsuccesses;
+ return true;
+ }
}
}
@@ -1293,9 +1316,40 @@ bool DependenceInfo::strongSIVtest(const SCEV *Coeff, const SCEV *SrcConst,
Result.DV[Level].Distance = Delta; // since X/1 == X
NewConstraint.setDistance(Delta, CurLoop);
} else {
- Result.Consistent = false;
- NewConstraint.setLine(Coeff, SE->getNegativeSCEV(Coeff),
- SE->getNegativeSCEV(Delta), CurLoop);
+ // Try symbolic division: Distance = Delta / Coeff.
+ if (const SCEV *Distance = SE->getUDivExactExpr(Delta, Coeff)) {
+ LLVM_DEBUG(dbgs() << "\t Symbolic distance = " << *Distance << "\n");
+ Result.DV[Level].Distance = Distance;
+ NewConstraint.setDistance(Distance, CurLoop);
+ } else {
+ // Cannot compute exact division - check if we can add runtime
+ // assumptions.
+ if (isa<SCEVUnknown>(Coeff) && !SE->isKnownNonZero(Coeff)) {
+ // Add runtime assumption that coefficient is non-zero for division.
+ const SCEV *Zero = SE->getZero(Coeff->getType());
+ const SCEVPredicate *NonZeroPred =
+ SE->getComparePredicate(ICmpInst::ICMP_NE, Coeff, Zero);
+ if (UnderRuntimeAssumptions) {
+ SmallVector<const SCEVPredicate *, 4> NewPreds(
+ Assumptions.getPredicates());
+ NewPreds.push_back(NonZeroPred);
+ const_cast<DependenceInfo *>(this)->Assumptions =
+ SCEVUnionPredicate(NewPreds, *SE);
+ LLVM_DEBUG(dbgs() << "\t Added runtime assumption: " << *Coeff
+ << " != 0 for symbolic division\n");
+ } else {
+ // Cannot add runtime assumptions, this test fails.
+ LLVM_DEBUG(dbgs()
+ << "\t Would need runtime assumption " << *Coeff
+ << " != 0 but not allowed. Failing this test.\n");
+ return false;
+ }
+ }
+
+ Result.Consistent = false;
+ NewConstraint.setLine(Coeff, SE->getNegativeSCEV(Coeff),
+ SE->getNegativeSCEV(Delta), CurLoop);
+ }
}
// maybe we can get a useful direction
@@ -3567,7 +3621,7 @@ bool DependenceInfo::invalidate(Function &F, const PreservedAnalyses &PA,
}
SCEVUnionPredicate DependenceInfo::getRuntimeAssumptions() const {
- return SCEVUnionPredicate(Assumptions, *SE);
+ return Assumptions;
}
// depends -
@@ -3584,7 +3638,12 @@ SCEVUnionPredicate DependenceInfo::getRuntimeAssumptions() const {
std::unique_ptr<Dependence>
DependenceInfo::depends(Instruction *Src, Instruction *Dst,
bool UnderRuntimeAssumptions) {
- SmallVector<const SCEVPredicate *, 4> Assume;
+ // Set the flag for whether we're allowed to add runtime assumptions.
+ this->UnderRuntimeAssumptions = UnderRuntimeAssumptions;
+
+ // Clear any previous assumptions
+ Assumptions = SCEVUnionPredicate({}, *SE);
+
bool PossiblyLoopIndependent = true;
if (Src == Dst)
PossiblyLoopIndependent = false;
@@ -3596,8 +3655,7 @@ DependenceInfo::depends(Instruction *Src, Instruction *Dst,
if (!isLoadOrStore(Src) || !isLoadOrStore(Dst)) {
// can only analyze simple loads and stores, i.e., no calls, invokes, etc.
LLVM_DEBUG(dbgs() << "can only handle simple loads and stores\n");
- return std::make_unique<Dependence>(Src, Dst,
- SCEVUnionPredicate(Assume, *SE));
+ return std::make_unique<Dependence>(Src, Dst, getRuntimeAssumptions());
}
const MemoryLocation &DstLoc = MemoryLocation::get(Dst);
@@ -3608,8 +3666,7 @@ DependenceInfo::depends(Instruction *Src, Instruction *Dst,
case AliasResult::PartialAlias:
// cannot analyse objects if we don't understand their aliasing.
LLVM_DEBUG(dbgs() << "can't analyze may or partial alias\n");
- return std::make_unique<Dependence>(Src, Dst,
- SCEVUnionPredicate(Assume, *SE));
+ return std::make_unique<Dependence>(Src, Dst, getRuntimeAssumptions());
case AliasResult::NoAlias:
// If the objects noalias, they are distinct, accesses are independent.
LLVM_DEBUG(dbgs() << "no alias\n");
@@ -3623,8 +3680,7 @@ DependenceInfo::depends(Instruction *Src, Instruction *Dst,
// The dependence test gets confused if the size of the memory accesses
// differ.
LLVM_DEBUG(dbgs() << "can't analyze must alias with different sizes\n");
- return std::make_unique<Dependence>(Src, Dst,
- SCEVUnionPredicate(Assume, *SE));
+ return std::make_unique<Dependence>(Src, Dst, getRuntimeAssumptions());
}
Value *SrcPtr = getLoadStorePointerOperand(Src);
@@ -3643,8 +3699,7 @@ DependenceInfo::depends(Instruction *Src, Instruction *Dst,
// We check this upfront so we don't crash in cases where getMinusSCEV()
// returns a SCEVCouldNotCompute.
LLVM_DEBUG(dbgs() << "can't analyze SCEV with different pointer base\n");
- return std::make_unique<Dependence>(Src, Dst,
- SCEVUnionPredicate(Assume, *SE));
+ return std::make_unique<Dependence>(Src, Dst, getRuntimeAssumptions());
}
// Even if the base pointers are the same, they may not be loop-invariant. It
@@ -3656,8 +3711,7 @@ DependenceInfo::depends(Instruction *Src, Instruction *Dst,
if (!isLoopInvariant(SrcBase, SrcLoop) ||
!isLoopInvariant(DstBase, DstLoop)) {
LLVM_DEBUG(dbgs() << "The base pointer is not loop invariant.\n");
- return std::make_unique<Dependence>(Src, Dst,
- SCEVUnionPredicate(Assume, *SE));
+ return std::make_unique<Dependence>(Src, Dst, getRuntimeAssumptions());
}
uint64_t EltSize = SrcLoc.Size.toRaw();
@@ -3665,35 +3719,40 @@ DependenceInfo::depends(Instruction *Src, Instruction *Dst,
const SCEV *DstEv = SE->getMinusSCEV(DstSCEV, DstBase);
// Check that memory access offsets are multiples of element sizes.
- if (!SE->isKnownMultipleOf(SrcEv, EltSize, Assume) ||
- !SE->isKnownMultipleOf(DstEv, EltSize, Assume)) {
+ SmallVector<const SCEVPredicate *, 4> TempAssumptions;
+ if (!SE->isKnownMultipleOf(SrcEv, EltSize, TempAssumptions) ||
+ !SE->isKnownMultipleOf(DstEv, EltSize, TempAssumptions)) {
LLVM_DEBUG(dbgs() << "can't analyze SCEV with different offsets\n");
- return std::make_unique<Dependence>(Src, Dst,
- SCEVUnionPredicate(Assume, *SE));
+ return std::make_unique<Dependence>(Src, Dst, getRuntimeAssumptions());
}
- if (!Assume.empty()) {
- if (!UnderRuntimeAssumptions)
- return std::make_unique<Dependence>(Src, Dst,
- SCEVUnionPredicate(Assume, *SE));
- // Add non-redundant assumptions.
- unsigned N = Assumptions.size();
- for (const SCEVPredicate *P : Assume) {
- bool Implied = false;
- for (unsigned I = 0; I != N && !Implied; I++)
- if (Assumptions[I]->implies(P, *SE))
- Implied = true;
- if (!Implied)
- Assumptions.push_back(P);
+ // Add any new assumptions from the isKnownMultipleOf calls
+ if (!TempAssumptions.empty()) {
+ if (UnderRuntimeAssumptions) {
+ SmallVector<const SCEVPredicate *, 4> NewPreds(
+ Assumptions.getPredicates());
+ NewPreds.append(TempAssumptions.begin(), TempAssumptions.end());
+ const_cast<DependenceInfo *>(this)->Assumptions =
+ SCEVUnionPredicate(NewPreds, *SE);
+ } else {
+ // Runtime assumptions needed but not allowed.
+ // Return confused dependence since we cannot proceed with precise
+ // analysis.
+ LLVM_DEBUG(dbgs() << "Runtime assumptions needed for offset analysis but "
+ "not allowed\n");
+ return std::make_unique<Dependence>(Src, Dst, getRuntimeAssumptions());
}
}
+ // Assert that we haven't added runtime assumptions when not allowed
+ assert(UnderRuntimeAssumptions || Assumptions.isAlwaysTrue());
+
establishNestingLevels(Src, Dst);
LLVM_DEBUG(dbgs() << " common nesting levels = " << CommonLevels << "\n");
LLVM_DEBUG(dbgs() << " maximum nesting levels = " << MaxLevels << "\n");
- FullDependence Result(Src, Dst, SCEVUnionPredicate(Assume, *SE),
- PossiblyLoopIndependent, CommonLevels);
+ FullDependence Result(Src, Dst, Assumptions, PossiblyLoopIndependent,
+ CommonLevels);
++TotalArrayPairs;
unsigned Pairs = 1;
@@ -4036,6 +4095,10 @@ DependenceInfo::depends(Instruction *Src, Instruction *Dst,
return nullptr;
}
+ // Assert that we haven't added runtime assumptions when not allowed
+ assert(UnderRuntimeAssumptions || Assumptions.isAlwaysTrue());
+
+ Result.Assumptions = getRuntimeAssumptions();
return std::make_unique<FullDependence>(std::move(Result));
}
diff --git a/llvm/test/Analysis/DependenceAnalysis/BasePtrBug.ll b/llvm/test/Analysis/DependenceAnalysis/BasePtrBug.ll
index 81e461a5e092d..8933c5fa6520a 100644
--- a/llvm/test/Analysis/DependenceAnalysis/BasePtrBug.ll
+++ b/llvm/test/Analysis/DependenceAnalysis/BasePtrBug.ll
@@ -18,7 +18,7 @@ define void @test1(ptr nocapture %A, ptr nocapture %B, i32 %N) #0 {
; CHECK-NEXT: Src: %0 = load i32, ptr %gep.0, align 4 --> Dst: %0 = load i32, ptr %gep.0, align 4
; CHECK-NEXT: da analyze - none!
; CHECK-NEXT: Src: %0 = load i32, ptr %gep.0, align 4 --> Dst: %1 = load i32, ptr %gep.1, align 4
-; CHECK-NEXT: da analyze - input [*|<]!
+; CHECK-NEXT: da analyze - consistent input [((-4 * (sext i32 %div to i64))<nsw> /u 4)|<]!
; CHECK-NEXT: Src: %0 = load i32, ptr %gep.0, align 4 --> Dst: store i32 %add, ptr %gep.B, align 4
; CHECK-NEXT: da analyze - confused!
; CHECK-NEXT: Src: %1 = load i32, ptr %gep.1, align 4 --> Dst: %1 = load i32, ptr %gep.1, align 4
diff --git a/llvm/test/Analysis/DependenceAnalysis/DADelin.ll b/llvm/test/Analysis/DependenceAnalysis/DADelin.ll
index 8f94a455d3724..6e50712d6b458 100644
--- a/llvm/test/Analysis/DependenceAnalysis/DADelin.ll
+++ b/llvm/test/Analysis/DependenceAnalysis/DADelin.ll
@@ -648,7 +648,7 @@ define void @coeff_may_negative(ptr %a, i32 %k) {
; CHECK-NEXT: Src: store i8 42, ptr %idx.0, align 1 --> Dst: store i8 42, ptr %idx.0, align 1
; CHECK-NEXT: da analyze - none!
; CHECK-NEXT: Src: store i8 42, ptr %idx.0, align 1 --> Dst: store i8 42, ptr %idx.1, align 1
-; CHECK-NEXT: da analyze - output [*|<]!
+; CHECK-NEXT: da analyze - consistent output [((-1 * %k) /u %k)|<]!
; CHECK-NEXT: Src: store i8 42, ptr %idx.1, align 1 --> Dst: store i8 42, ptr %idx.1, align 1
; CHECK-NEXT: da analyze - none!
;
@@ -687,7 +687,7 @@ define void @coeff_positive(ptr %a, i32 %k) {
; CHECK-NEXT: Src: store i8 42, ptr %idx.0, align 1 --> Dst: store i8 42, ptr %idx.0, align 1
; CHECK-NEXT: da analyze - none!
; CHECK-NEXT: Src: store i8 42, ptr %idx.0, align 1 --> Dst: store i8 42, ptr %idx.1, align 1
-; CHECK-NEXT: da analyze - output [*|<]!
+; CHECK-NEXT: da analyze - consistent output [((-1 * %k) /u %k)|<]!
; CHECK-NEXT: Src: store i8 42, ptr %idx.1, align 1 --> Dst: store i8 42, ptr %idx.1, align 1
; CHECK-NEXT: da analyze - none!
;
diff --git a/llvm/test/Analysis/DependenceAnalysis/DifferentOffsets.ll b/llvm/test/Analysis/DependenceAnalysis/DifferentOffsets.ll
index d9ccea55dd478..719a62a3d5113 100644
--- a/llvm/test/Analysis/DependenceAnalysis/DifferentOffsets.ll
+++ b/llvm/test/Analysis/DependenceAnalysis/DifferentOffsets.ll
@@ -34,8 +34,6 @@ define i32 @alias_with_parametric_offset(ptr nocapture %A, i64 %n) {
; CHECK-NEXT: Equal predicate: (zext i2 (trunc i64 %n to i2) to i64) == 0
; CHECK-NEXT: Src: %0 = load i32, ptr %A, align 1 --> Dst: %0 = load i32, ptr %A, align 1
; CHECK-NEXT: da analyze - none!
-; CHECK-NEXT: Runtime Assumptions:
-; CHECK-NEXT: Equal predicate: (zext i2 (trunc i64 %n to i2) to i64) == 0
;
entry:
%arrayidx = getelementptr inbounds i8, ptr %A, i64 %n
@@ -56,7 +54,6 @@ define i32 @alias_with_parametric_expr(ptr nocapture %A, i64 %n, i64 %m) {
; CHECK-NEXT: Src: %0 = load i32, ptr %arrayidx1, align 1 --> Dst: %0 = load i32, ptr %arrayidx1, align 1
; CHECK-NEXT: da analyze - none!
; CHECK-NEXT: Runtime Assumptions:
-; CHECK-NEXT: Equal predicate: (zext i2 ((trunc i64 %m to i2) + (-2 * (trunc i64 %n to i2))) to i64) == 0
; CHECK-NEXT: Equal predicate: (zext i2 (-2 + (trunc i64 %m to i2)) to i64) == 0
;
entry:
@@ -81,8 +78,6 @@ define i32 @gep_i8_vs_i32(ptr nocapture %A, i64 %n, i64 %m) {
; CHECK-NEXT: Equal predicate: (zext i2 (trunc i64 %n to i2) to i64) == 0
; CHECK-NEXT: Src: store i32 42, ptr %arrayidx1, align 4 --> Dst: store i32 42, ptr %arrayidx1, align 4
; CHECK-NEXT: da analyze - none!
-; CHECK-NEXT: Runtime Assumptions:
-; CHECK-NEXT: Equal predicate: (zext i2 (trunc i64 %n to i2) to i64) == 0
;
entry:
%arrayidx0 = getelementptr inbounds i8, ptr %A, i64 %n
diff --git a/llvm/test/Analysis/DependenceAnalysis/MIVCheckConst.ll b/llvm/test/Analysis/DependenceAnalysis/MIVCheckConst.ll
index b498d70648bad..bb6d2d7c4c8f2 100644
--- a/llvm/test/Analysis/DependenceAnalysis/MIVCheckConst.ll
+++ b/llvm/test/Analysis/DependenceAnalysis/MIVCheckConst.ll
@@ -50,9 +50,6 @@ define void @test(ptr %A, ptr %B, i1 %arg, i32 %n, i32 %m) #0 align 2 {
; CHECK-NEXT: Equal predicate: (8 * (zext i4 (trunc i32 %v1 to i4) to i32))<nuw><nsw> == 0
; CHECK-NEXT: Src: %v32 = load <32 x i32>, ptr %v30, align 128 --> Dst: %v32 = load <32 x i32>, ptr %v30, align 128
; CHECK-NEXT: da analyze - consistent input [0 S S]!
-; CHECK-NEXT: Runtime Assumptions:
-; CHECK-NEXT: Equal predicate: (zext i7 (4 * (trunc i32 %v1 to i7) * (1 + (trunc i32 %n to i7))) to i32) == 0
-; CHECK-NEXT: Equal predicate: (8 * (zext i4 (trunc i32 %v1 to i4) to i32))<nuw><nsw> == 0
;
entry:
%v1 = load i32, ptr %B, align 4
diff --git a/llvm/test/Analysis/DependenceAnalysis/PR149977.ll b/llvm/test/Analysis/DependenceAnalysis/PR149977.ll
new file mode 100644
index 0000000000000..1576fe99d8767
--- /dev/null
+++ b/llvm/test/Analysis/DependenceAnalysis/PR149977.ll
@@ -0,0 +1,45 @@
+; NOTE: Assertions have been autogenerated by utils/update_analyze_test_checks.py UTC_ARGS: --version 5
+; RUN: opt < %s -disable-output "-passes=print<da>" 2>&1 | FileCheck %s
+
+; Test case for GitHub issue #149977: Strong SIV test with symbolic coefficients and deltas
+; The issue was that the bound constraint check was overly conservative with symbolic expressions,
+; causing valid dependencies to be rejected before reaching the division logic.
+;
+; Mathematical analysis:
+; - Access patterns: a[-k*i] vs a[-k*i + (2*k + 1)]
+; - Strong SIV equation: -k*(i2-i1) = (2*k + 1)
+; - Distance: (2*k + 1)/k
+; - For k=-1: distance = -1/-1 = 1 (clear dependence)
+
+define void @f(ptr %a, i64 %k) {
+; CHECK-LABEL: 'f'
+; CHECK-NEXT: Src: store i8 42, ptr %idx.0, align 1 --> Dst: store i8 42, ptr %idx.0, align 1
+; CHECK-NEXT: da analyze - none!
+; CHECK-NEXT: Src: store i8 42, ptr %idx.0, align 1 --> Dst: store i8 42, ptr %idx.1, align 1
+; CHECK-NEXT: da analyze - consistent output [((-1 + (-2 * %k)) /u (-1 * %k))]!
+; CHECK-NEXT: Runtime Assumptions:
+; CHECK-NEXT: Compare predicate: (1 + (2 * %k))<nuw><nsw> sle) (2 * %k)
+; CHECK-NEXT: Src: store i8 42, ptr %idx.1, align 1 --> Dst: store i8 42, ptr %idx.1, align 1
+; CHECK-NEXT: da analyze - none!
+;
+entry:
+ %mk = sub i64 0, %k ; mk = -k
+ %kk = mul i64 %k, 2 ; kk = 2*k
+ %kk.inc = add i64 1, %kk ; kk.inc = 2*k + 1
+ br label %loop
+
+loop:
+ %i = phi i64 [ 0, %entry ], [ %i.next, %loop ]
+ %subscript.0 = mul i64 %mk, %i ; -k * i
+ %subscript.1 = add i64 %subscript.0, %kk.inc ; -k * i + (2*k + 1)
+ %idx.0 = getelementptr i8, ptr %a, i64 %subscript.0
+ %idx.1 = getelementptr i8, ptr %a, i64 %subscript.1
+ store i8 42, ptr %idx.0
+ store i8 42, ptr %idx.1
+ %i.next = add i64 %i, 1
+ %cond.exit = icmp eq i64 %i.next, ...
[truncated]
|
sebpop
requested review from
Meinersbur,
kasuga-fj,
madhur13490,
sjoerdmeijer and
sushgokh
kasuga-fj
reviewed
Sep 10, 2025
Contributor
There was a problem hiding this comment.
What I was trying to emphasize in that issue is simpler: the way to compute the absolute value is incorrect. Currently both AbsDelta and AbsCoeff are computed as follows:
const SCEV *AbsXXX = SE->isKnownNonNegative(XXX) ? XXX : SE->getNegativeSCEV(XXX);If the sign of XXX is unknown, isKnownNonNegative(XXX) will return false, resulting in AbsXXX being -XXX, which can be negative. The correct way to compute the absolute value is using ScalarEvolution::getAbsExpr.
Contributor
Author
I agree. The current way of using SE->isKnownNonNegative may be unsafe. Do you have a patch for this? |
Contributor
|
Not yet. Feel free to go ahead if you’d like. |
sushgokh
reviewed
Sep 15, 2025
| // Check if this involves symbolic expressions where we might be too | ||
| // conservative. | ||
| if (isa<SCEVUnknown>(Delta) || isa<SCEVUnknown>(Coeff) || | ||
| !isa<SCEVConstant>(AbsDelta) || !isa<SCEVConstant>(Product)) { |
Contributor
There was a problem hiding this comment.
if( isa<SCEVUnknown>(Delta) || !isa<SCEVConstant>(Product) )
should be sufficient ?
| // For symbolic expressions, add runtime assumption rather than | ||
| // rejecting. | ||
| const SCEVPredicate *BoundPred = | ||
| SE->getComparePredicate(ICmpInst::ICMP_SLE, AbsDelta, Product); |
Contributor
There was a problem hiding this comment.
push this inside if(){...}
| Assumptions.getPredicates()); | ||
| NewPreds.push_back(NonZeroPred); | ||
| const_cast<DependenceInfo *>(this)->Assumptions = | ||
| SCEVUnionPredicate(NewPreds, *SE); |
Contributor
There was a problem hiding this comment.
why are we adding constraint once again when we have already added below constraint?
const SCEVPredicate *BoundPred =
SE->getComparePredicate(ICmpInst::ICMP_SLE, AbsDelta, Product);
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes GitHub issue #149977 where Strong SIV test incorrectly rejected
dependencies with symbolic coefficients and deltas due to overly conservative
bound checking.
Root cause: The bound constraint check |Delta| > UpperBound * |Coeff| would
prematurely reject dependencies when SCEV couldn't prove the relationship
definitively for symbolic expressions, preventing the analysis from reaching
the division logic.
Solution:
runtime assumptions when SCEV cannot determine the relationship.
This enables precise dependence analysis for cases like:
Test case validates: